chore: CRP-2960 Align spec and implementation of encrypted_chat ratcheting#281
Merged
chore: CRP-2960 Align spec and implementation of encrypted_chat ratcheting#281
Conversation
…eting
This fixes several areas where the spec and implementation of ratcheting
in the encrypted_chat example did not align:
1) The spec for evolving the state used the current epoch to define the domain
separator, then returned a new state with the new key and the next epoch. The
implementation instead had a bug where it incremented twice, once at the
start and once at the end. So the key would be derived using the next epoch
id n, but then have an epoch id of n+1. Confusing! Change both: specify that
the next (new) epoch is the value that is added to the domain separator.
2) The implementation adds -example to the domain separator strings while the
spec does not. Minor issue but might as well be consistent.
3) evolveTo did not evolve states ... it first checked if the requested epoch is
less than or equal to the current epoch, in which case it returns
early. Otherwise it looped in this fashion
while(requestedEpoch < currentEpoch)
which would never occur since the function would have already have returned
in this case. I renamed the relevant argument to desiredEpoch since I think
that makes it easier to read/reason about what is happening.
4) Fixed CacheableSymmetricRatchetState::peekAtEpoch. It pre-incremented the
epoch before cloning the structure, but this puts it out of sync with the
correct epoch and so different keys would be derived. This function doesn't
seem to be used, which is probably why this wasn't noticed. It may be better
to remove this function entirely.
fspreiss
approved these changes
Jan 19, 2026
Contributor
fspreiss
left a comment
There was a problem hiding this comment.
Thanks for the fixes, @randombit!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes several areas where the spec and implementation of ratcheting in the encrypted_chat example did not align:
The spec for evolving the state used the current epoch to define the domain
separator, then returned a new state with the new key and the next epoch. The
implementation instead had a bug where it incremented twice, once at the
start and once at the end. So the key would be derived using the next epoch
id n, but then have an epoch id of n+1. Confusing! Change both: specify that
the next (new) epoch is the value that is added to the domain separator.
The implementation adds -example to the domain separator strings while the
spec does not. Minor issue but might as well be consistent.
evolveTo did not evolve states ... it first checked if the requested epoch is
less than or equal to the current epoch, in which case it returns
early. Otherwise it looped in this fashion
while(requestedEpoch < currentEpoch)
which would never occur since the function would have already have returned
in this case. I renamed the relevant argument to desiredEpoch since I think
that makes it easier to read/reason about what is happening.
Fixed CacheableSymmetricRatchetState::peekAtEpoch. It pre-incremented the
epoch before cloning the structure, but this puts it out of sync with the
correct epoch and so different keys would be derived. This function doesn't
seem to be used, which is probably why this wasn't noticed. It may be better
to remove this function entirely.